Skip to content

[JULES] Scheduled Maintenance: Pattern Value Refactor - #504

Closed
logbie wants to merge 1 commit into
mainfrom
maintenance-refactor-pattern-expect-17883455864191053605
Closed

logbie wants to merge 1 commit into
mainfrom
maintenance-refactor-pattern-expect-17883455864191053605

Conversation

@logbie

@logbie logbie commented May 15, 2026

Copy link
Copy Markdown
Collaborator

Summary of Changes

  • The Issue: Manual match blocks were being used repeatedly across src/stdlib/pattern.rs to extract Value::Text and Value::Pattern from function arguments, duplicating error handling and extraction logic.
  • The Rational: Improved maintainability, eliminated redundancy, and standardized error messaging.
  • The Solution: Added a new macro-generated expect_pattern to src/stdlib/helpers.rs and refactored multiple functions in src/stdlib/pattern.rs (pattern_matches_native, pattern_find_native, pattern_find_all_native, native_pattern_replace, native_pattern_split) to use the expect_text and expect_pattern helpers instead of manual pattern matching.

Verification Checklist

  • cargo fmt executed and passed.
  • cargo clippy returned no warnings or errors.
  • All cargo test suites passed (100% success rate).

PR created automatically by Jules for task 17883455864191053605 started by @logbie


Open in Devin Review

Summary by CodeRabbit

  • Refactor

    • Consolidated pattern validation logic and improved error messages across pattern-related operations, including matching, finding, replacement, and splitting functions.
  • Tests

    • Updated test assertions to verify more specific and descriptive error messages when pattern functions receive incorrect argument types.

Review Change Stack

Co-authored-by: logbie <1138960+logbie@users.noreply.github.com>
@google-labs-jules

Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

Copilot AI review requested due to automatic review settings May 15, 2026 09:35
@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f8a82919-4c77-459b-920d-878b7442e54e

📥 Commits

Reviewing files that changed from the base of the PR and between 68d08ee and e45023c.

📒 Files selected for processing (3)
  • src/stdlib/helpers.rs
  • src/stdlib/pattern.rs
  • src/stdlib/pattern_test.rs

📝 Walkthrough

Walkthrough

This PR introduces a new expect_pattern helper extractor and refactors five pattern native functions and their tests to standardize argument validation. Manual Value destructuring and custom error reporting are replaced with reusable expect_text and expect_pattern helpers, producing more consistent error messages.

Changes

Pattern helper and native function consolidation

Layer / File(s) Summary
New expect_pattern extractor
src/stdlib/helpers.rs
Added expect_pattern function that validates a Value is a Pattern variant, returning Rc<CompiledPattern> on success or RuntimeError with type mismatch details on failure.
Refactor pattern native functions to use helpers
src/stdlib/pattern.rs
pattern_matches_native, pattern_find_native, pattern_find_all_native, native_pattern_replace, and native_pattern_split now extract text and pattern arguments via shared expect_text and expect_pattern helpers, replacing manual Value pattern matching. Result shapes remain unchanged.
Update test assertion for refactored error messages
src/stdlib/pattern_test.rs
test_pattern_matches_native_wrong_first_arg_type now validates the specific error substring "Expected text, got Number" produced by the standardized helper extraction.

Possibly related PRs

  • WebFirstLanguage/wfl#310: Extends src/stdlib/helpers.rs with expect_pattern and refactors pattern functions to use shared expect_text/expect_pattern extractors.
  • WebFirstLanguage/wfl#427: Adds and standardizes typed expect_* extractors in src/stdlib/helpers.rs through macro generation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A pattern extracts what it seeks,
No more handwritten error-speak,
Type-checking helpers stand in line,
Each native function now feels fine,
Consistent checks, no mess, no strife!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: a refactoring of pattern value extraction logic to use helper functions instead of manual matching.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch maintenance-refactor-pattern-expect-17883455864191053605

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors pattern stdlib argument extraction to use shared helper functions, reducing duplicated Value::Text/Value::Pattern matching and standardizing type error messages.

Changes:

  • Adds a macro-generated expect_pattern helper for extracting Value::Pattern.
  • Refactors pattern matching/find/replace/split natives to use expect_text and expect_pattern.
  • Updates the affected pattern test expectation for the standardized text type error.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/stdlib/helpers.rs Adds the reusable expect_pattern extraction helper.
src/stdlib/pattern.rs Replaces repeated manual argument type matching with helper calls.
src/stdlib/pattern_test.rs Updates the wrong-argument-type assertion to match standardized helper output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

@logbie logbie closed this May 22, 2026
@logbie
logbie deleted the maintenance-refactor-pattern-expect-17883455864191053605 branch June 19, 2026 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants